[core] Resolve oneOf discriminator property type from child schemas#24172
[core] Resolve oneOf discriminator property type from child schemas#24172Ignacio-Vidal wants to merge 1 commit into
Conversation
3a3cbe2 to
0a6c7f0
Compare
|
@Mattias-Sehlstedt - This is an enhancement to your recent contribution 439f608 (introduce DiscriminatorUtils). Could you review it please? @wing328 - Could you review i? Given you approved 439f608 too |
4532046 to
02c7380
Compare
There was a problem hiding this comment.
1 issue found and verified against the latest diff
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
4483ce2 to
310c3f0
Compare
|
Hi, see #24158 and #24156 for similar changes. For your change I would say that the same as in #24156 (comment) applies (i.e., is it really necessary to do two recursive passes?). |
getDiscriminatorPropertyType resolved the discriminator property's type only from the oneOf/anyOf schema's own properties, defaulting to "string" when the property was not declared there. For a pure oneOf interface whose discriminator property lives on a shared base that the children inherit via allOf, this produced a String getter type that clashed with the enum the subtypes actually expose. Add the resolution to DiscriminatorUtils (alongside the existing getDiscriminatorPropertyType / getDiscriminatorSchema): getDiscriminatorPropertyType now falls back to the mapped child schemas, chasing the discriminator property through each child's allOf members (getDiscriminatorPropertyTypeFromChildren / getDiscriminatorSchemaDeep). The allOf descent tracks visited schemas to guard against a cyclic allOf composition recursing infinitely. The fallback only fires when the own-properties lookup is empty, so the type now resolves to the enum ref (e.g. PetType) when a child declares the discriminator property as a $ref, and otherwise still falls back to "string". DefaultCodegen.getDiscriminatorPropertyType becomes a thin binding that maps the resolved schema ref name to a generator-specific model name and applies the "string" default, as those depend on this codegen's instance state. The first resolution branch is unchanged, so existing specs are unaffected: regenerating the full sample suite produces no diffs.
310c3f0 to
a0934fe
Compare
I didn't see there were 2 more PRs addressing the same bug, happy to close mine in favour of #24158 and #24156 |
What
getDiscriminatorPropertyTyperesolved the discriminator property's type only from theoneOf/anyOfschema's own properties, defaulting to"string"otherwise. For a pureoneOf/anyOfinterface whose discriminator property lives on a shared base the children inherit viaallOf, this produced aStringgetter that clashed with the enum the subtypes actually expose.This adds a fallback (in
DiscriminatorUtils) that resolves the type from the mapped child schemas, chasing the property through each child'sallOfmembers. It only fires when the previous code would have returned"string", so existing specs are unaffected — regenerating the full sample suite produces no diffs.Scenarios covered
$refon the schema's own properties (unchanged path)PetType)DiscriminatorUtilsTest,DefaultCodegenTestoneOfchildren inherit viaallOfPetType(from children)DiscriminatorUtilsTest,DefaultCodegenTestanyOfchildren inherit viaallOfPetType(from children)DiscriminatorUtilsTestallOfPetType(recursive descent)DiscriminatorUtilsTeststring(no$ref, no enum)"string"(fallback, unchanged)DiscriminatorUtilsTest,DefaultCodegenTestSimples example where the
discriminatoris in thebaseobject that is separate from the schema that containsoneOfPR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
Summary by cubic
Fixes discriminator type resolution for oneOf/anyOf interfaces by deriving the property type from child schemas (via allOf) with cycle-safe recursion. Getter types now use the correct enum (e.g., PetType) instead of "string", avoiding interface vs subtype mismatches.
DiscriminatorUtils.getDiscriminatorPropertyType(OpenAPI, ...)to first check the schema’s own$ref, then fall back to oneOf/anyOf children viagetDiscriminatorPropertyTypeFromChildrenand recursivegetDiscriminatorSchemaDeep(supports multi-level allOf with a visited guard for cycles).DefaultCodegen.getDiscriminatorPropertyTypeto passopenAPI, map to model names, and keep the "string" fallback.DiscriminatorUtilsTest, expandedDefaultCodegenTest, and new YAMLs (oneOf/anyOf, nested base, cyclic, string fallback). Regenerating samples shows no diffs.Written for commit a0934fe. Summary will update on new commits.